CASSANDRA-20918 Add cursor-based low allocation optimized compaction implementation #4402

nitsanw · 2025-09-29T15:22:19Z

patch by Nitsan Wakart; reviewed by TBD for CASSANDRA-TBD

EPPv1 - all new code
Cursor compaction integration
JMH benchmarks for compaction and cursor impls
EPPv1 - New tests
Existing tests tweaks for new code
[revert?] change the default partitioner to expand testing of new code
[revert?] test data used some benchmarks
[revert?] jmh tweak GC settings for stability
[revert?] javadoc typos, marking unused params
[revert?] clarifying comment
[revert?] toString improvement
[revert?] remove spurious keywords
[revert?] marking metadata collection
[revert?] cursor verifier
Exclude SAI and counter column
Exclude BTI and legacy versions
Temporarily skip very long running test

Thanks for sending a pull request! Here are some tips if you're new here:

Ensure you have added or run the appropriate tests for your PR.
Be sure to keep the PR description updated to reflect all changes.
Write your PR title to summarize what this PR proposes.
If possible, provide a concise example to reproduce the issue for a faster review.
Read our contributor guidelines
If you're making a documentation change, see our guide to documentation contribution

Commit messages should follow the following format:

<One sentence description, usually Jira title or CHANGES.txt summary>

<Optional lengthier description (context on patch)>

patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####

Co-authored-by: Name1 <email1>
Co-authored-by: Name2 <email2>

The Cassandra Jira

patch by Nitsan Wakart; reviewed by TBD for CASSANDRA-TBD - EPPv1 - all new code - Cursor compaction integration - JMH benchmarks for compaction and cursor impls - EPPv1 - New tests - Existing tests tweaks for new code - [revert?] change the default partitioner to expand testing of new code - [revert?] test data used some benchmarks - [revert?] jmh tweak GC settings for stability - [revert?] javadoc typos, marking unused params - [revert?] clarifying comment - [revert?] toString improvement - [revert?] remove spurious keywords - [revert?] marking metadata collection - [revert?] cursor verifier - Exclude SAI and counter column - Exclude BTI and legacy versions - Temporarily skip very long running test

src/java/org/apache/cassandra/db/compaction/CompactionTask.java

src/java/org/apache/cassandra/db/rows/Rows.java

src/java/org/apache/cassandra/io/util/RandomAccessReader.java

src/java/org/apache/cassandra/db/compaction/CompactionTask.java

blambov

It must be stated that this approach that bundles all the steps of the processing in one single file will be quite difficult to maintain and keep in sync with the combination of iterators and transformations that we use in other parts of the code such as the query path. However, once we have reached a point of stability for a piece of functionality where we do not expect it to change significantly for a long time, it does makes sense to unpack the code and present it in a way that makes its execution as direct as possible, and this patch is a good such representation of the compaction process.

Personally, I am very unhappy about switching to mutable, pooled and reused objects, which are significantly more unwieldy and error prone, especially in contexts where concurrent access can occur. It seems this is becoming a necessity if we need to achieve acceptable performance with the current state of our heap usage, but we still need to very carefully separate the mutable versions of concepts from the immutable ones used throughout the code base. Suddenly making a DeletionTime mutable is not an acceptable change.

First batch of targeted comments below, mainly going over CompactionCursor.java.

src/java/org/apache/cassandra/db/compaction/CompactionTask.java

src/java/org/apache/cassandra/db/compaction/CompactionCursor.java

blambov

Next batch of comments.

src/java/org/apache/cassandra/db/ClusteringPrefix.java

src/java/org/apache/cassandra/db/ClusteringComparator.java

blambov · 2025-10-13T13:30:00Z

src/java/org/apache/cassandra/io/sstable/UnfilteredDescriptor.java

+        }
+        if (!UnfilteredSerializer.hasAllColumns(flags))
+        {
+            // TODO: re-implement GC free


DataStax's branch has an implementation of it.

src/java/org/apache/cassandra/io/sstable/format/SortedTableWriter.java

src/java/org/apache/cassandra/db/compaction/writers/DefaultCompactionWriter.java

src/java/org/apache/cassandra/io/sstable/metadata/MetadataCollector.java

src/java/org/apache/cassandra/db/ClusteringComparator.java

…erties/Config

… elsewhere using seek

blambov

Next batch of comments.

src/java/org/apache/cassandra/utils/vint/VIntCoding.java

blambov · 2025-10-14T09:13:35Z

src/java/org/apache/cassandra/io/util/ReusableLongToken.java

+import org.apache.cassandra.dht.Murmur3Partitioner;
+import org.jctools.util.UnsafeAccess;
+
+public class ReusableLongToken extends Murmur3Partitioner.LongToken


Nit: This shouldn't need to be public.

blambov · 2025-10-14T09:25:14Z

src/java/org/apache/cassandra/io/util/ReusableDecoratedKey.java

+import org.apache.cassandra.utils.ByteArrayUtil;
+import org.apache.cassandra.utils.ByteBufferUtil;
+
+public class ReusableDecoratedKey extends DecoratedKey


This class is pretty hacky. It shouldn't be hard to move the support for reusable tokens to the partitioner (throwing exceptions for all except Murmur and local).

src/java/org/apache/cassandra/db/LivenessInfo.java

src/java/org/apache/cassandra/io/sstable/AbstractSSTableSimpleWriter.java

src/java/org/apache/cassandra/io/sstable/SSTableCursorReader.java

blambov · 2025-10-14T14:25:49Z

src/java/org/apache/cassandra/io/sstable/SSTableCursorWriter.java

+        appendBIGIndex(partitionKey, partitionKeyLength, partitionStart, headerLength, partitionDeletionTime, partitionEnd);
+    }
+
+    private void appendBIGIndex(byte[] key, int keyLength, long partitionStart, int headerLength, DeletionTime partitionDeletionTime, long partitionEnd) throws IOException


Is it not easy to modify and reuse the index building code from BigFormatPartitionWriter? The duplication here seems quite unnecessary.

blambov · 2025-10-14T14:26:56Z

src/java/org/apache/cassandra/io/sstable/SSTableCursorWriter.java

+    private final int indexBlockThreshold;
+
+
+    private SSTableCursorWriter(


This class should be split into a common SortedTableCursorWriter, with format-specific subclasses that instantiate the index builders it uses, and placed into the correct per-format packages.

Currently only BIG is supported, so I held off here. I think the index code split can be postponed to the time when the BTI format is supported. It may also be interesting to explore splitting the SSTable write phase and indexing phase so as to allow more flexibility in composing the phases (e.g. index on the fly/index per partition write/index at end of write, parallel index pass etc).

…nMerge` and return true when partitions remain

Javadoc for bubbleInsertToPreSorted

Simplify deletion merging loop, and some refactoring of names

src/java/org/apache/cassandra/config/Config.java

src/java/org/apache/cassandra/db/compaction/CompactionTask.java

src/java/org/apache/cassandra/db/compaction/CompactionCursor.java

blambov · 2025-11-03T15:45:27Z

src/java/org/apache/cassandra/db/compaction/AbstractCompactionPipeline.java

+    {
        if (DatabaseDescriptor.enableCursorCompaction()) {
-            try {
+            if (CompactionCursor.isSupported(scanners, controller))


We should still log whether we are doing cursor compaction.

This is now debug logged in the isSupported method, with a reason for the rejection. Do you think further logging is required?

src/java/org/apache/cassandra/db/compaction/CompactionCursor.java

…sMaxStart check

src/java/org/apache/cassandra/db/compaction/CompactionCursor.java

…ging

… on done

Nitsan Wakart and others added 2 commits August 28, 2025 14:26

Merge branch 'trunk' into compaction-work-pr-prep

759dc2c

nitsanw changed the title ~~Add cursor based optimized compaction path (WIP)~~ CASSANDRA-20918 Add cursor-based low allocation optimized compaction implementation Sep 29, 2025

blambov self-requested a review September 30, 2025 07:40

aratno reviewed Oct 8, 2025

View reviewed changes

src/java/org/apache/cassandra/db/compaction/CompactionTask.java Outdated Show resolved Hide resolved

src/java/org/apache/cassandra/db/rows/Rows.java Outdated Show resolved Hide resolved

src/java/org/apache/cassandra/io/util/RandomAccessReader.java Outdated Show resolved Hide resolved

blambov reviewed Oct 9, 2025

View reviewed changes

src/java/org/apache/cassandra/db/compaction/CompactionTask.java Outdated Show resolved Hide resolved

src/java/org/apache/cassandra/db/compaction/CompactionTask.java Outdated Show resolved Hide resolved

Fix MEGABYTE constant

d75d59c

blambov requested changes Oct 10, 2025

View reviewed changes

blambov reviewed Oct 13, 2025

View reviewed changes

Nitsan Wakart added 6 commits October 14, 2025 14:40

Fix 0x11/0x10 should be 0b11/0b10

35ea514

Introduce ENABLE_CURSOR_COMPACTION controls via CassandraRelevantProp…

33b9e47

…erties/Config

Revert matched comments left to track stats tracking

d46511d

Revert change to RandomAccessReader and match row skipping logic from…

b417274

… elsewhere using seek

Improve CompactionCursor javadoc

85379f1

Extract isSupported from constructor

097b995

blambov reviewed Oct 14, 2025

View reviewed changes

Nitsan Wakart added 14 commits October 15, 2025 10:51

Fix Trasnactions/Transformations

1af0f0e

Add comment and rename sortForPartitionMerge -> `prepareForPartitio…

7576864

…nMerge` and return true when partitions remain

Typo: preturbed -> perturbed

2205e48

Javadoc for bubbleInsertToPreSorted and minor refactor

dea15c9

Javadoc for bubbleInsertToPreSorted

Typo: passed -> past

5d200ef

Remove redundant TODOs

1a1c761

Revert 'unused' params

b9dc6db

Rename ElementDescriptor -> UnfilteredDescriptor (and fallout)

5accdd5

Remove unused parameter

17be5f4

Remove unused method

3b0f0a9

Fix indentation

ae65b7e

Move SSTableCursorPipeUtil to benchmarks

62837f9

Rename partitionLength back to finishResult and clarify comment

4064d32

Remove unused methods

797ace1

Nitsan Wakart added 14 commits October 21, 2025 15:26

Improve bubbleInsertElementToPreSorted, delay element insert

855a740

Remove redundant cursor status check

beef9ad

Simplify deletion merging loop, clarify partitionDeletion variable names

b19b959

Simplify deletion merging loop, and some refactoring of names

Neaten up SSTableCursorReader

f5718b3

Dead code removal

87a1c1c

Revert making classes public

ff5eff1

Transform LivenessInfo an interface

ed48fe5

Fix javadoc

a8bc083

Fix intellij warnings

be8b4e3

Explicitly split DeletionTime implementations

011213a

Rely on nextElementEquality in findMergeLimit

4d74fce

Refactor prepareAndSortForMerge code

b9f5802

Move merge limit == 0 out of mergeRows

308a3b3

Add TODO for clustering read/skip

562e437

blambov reviewed Nov 3, 2025

View reviewed changes

Nitsan Wakart added 2 commits November 4, 2025 08:32

Simplify ClusteringComparator code and remove redundant code

37104f6

Refactor ClusteringDescriptor for clarity and fix MetadataCollector i…

d7abcb6

…sMaxStart check

blambov reviewed Nov 4, 2025

View reviewed changes

src/java/org/apache/cassandra/db/compaction/CompactionCursor.java Outdated Show resolved Hide resolved

Nitsan Wakart added 10 commits November 4, 2025 13:12

Add comment regarding boundary

8e24365

Split checkNextFlags by context and tidy-up

bc27a3d

Use RuntimeException instead of ISE

16f2eee

Improve CompactionCursor javadoc

80007eb

Remove "system" table check

cf77915

Check output format for isSupported, improve unsupported reason log…

ff8ab58

…ging

Move partition start out of maybePurgedOutputDeletion

647f3b6

Improve prepareAndSortForPartitionMerge javadoc, avoid repeated reset…

35af776

… on done

Change option name enable_cursor_compaction -> cursor_compaction_enabled

2e8dff9

Add option cursor_compaction_enabled to testlist-oa

d2b23d6

		private final int indexBlockThreshold;


		private SSTableCursorWriter(

CASSANDRA-20918 Add cursor-based low allocation optimized compaction implementation #4402

Are you sure you want to change the base?

CASSANDRA-20918 Add cursor-based low allocation optimized compaction implementation #4402

Conversation

nitsanw commented Sep 29, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

blambov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

blambov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

blambov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants